-
Notifications
You must be signed in to change notification settings - Fork 467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API Updates #1328
API Updates #1328
Conversation
Something is up with our codegen and removed Charge#ShippingDetails. Looking... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments early but will defer to @richardm-stripe for final review
Phone string `json:"phone"` | ||
TrackingNumber string `json:"tracking_number"` | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whis is this disappearing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-added - it should've been manually specified in address.go.
issuingcard "github.com/stripe/stripe-go/v72/issuing/card" | ||
"github.com/stripe/stripe-go/v72/issuing/cardholder" | ||
issuingcardholder "github.com/stripe/stripe-go/v72/issuing/cardholder" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are those changes breaking changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was actually merged in #1323 .
type ReviewReason string | ||
|
||
// Deprecated: we preserve this name for backwards-compatibility, prefer `ReviewReason`. | ||
type ReviewReasonType = ReviewReason |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a breaking change or are type aliasing safe, same for the one below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type aliasing here should be safe.
@@ -86,11 +97,11 @@ type Review struct { | |||
Open bool `json:"open"` | |||
OpenedReason ReviewOpenedReason `json:"opened_reason"` | |||
PaymentIntent *PaymentIntent `json:"payment_intent"` | |||
Reason ReviewReasonType `json:"reason"` | |||
Reason ReviewReason `json:"reason"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type aliasing here should be safe.
I've ran into this with Richard at some point and his suggestion was to move |
3b2420e
to
d0d448e
Compare
type ChargePaymentMethodDetailsSepaCreditTransfer struct { | ||
BankName string `json:"bank_name"` | ||
Bic string `json:"bic"` | ||
Iban string `json:"iban"` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type ChargePaymentMethodDetailsSepaCreditTransfer struct { | |
BankName string `json:"bank_name"` | |
Bic string `json:"bic"` | |
Iban string `json:"iban"` | |
} | |
type ChargePaymentMethodDetailsSepaCreditTransfer struct { | |
BankName string `json:"bank_name"` | |
BIC string `json:"bic"` | |
IBAN string `json:"iban"` | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually nevermind, we don't capitalize these acronyms anywhere and should be consistent with elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually nevermind, we don't capitalize these acronyms anywhere and should be consistent with elsewhere.
Codegen for openapi 10ebdb9.
r? @richardm-stripe
cc @stripe/api-libraries
Changelog
au_arn
InteracPresent
onChargePaymentMethodDetails
SepaCreditTransfer
onChargePaymentMethodDetails
ShippingDetails
intoaddress.go
Object
andOrder
toCharge
ReviewReasonType
enum toReviewReason
but added a type alias to preserve backwards compatibility